Skip to content

Theopendraft#4331

Open
aladdyn-io wants to merge 8 commits into
umami-software:masterfrom
aladdyn-io:theopendraft
Open

Theopendraft#4331
aladdyn-io wants to merge 8 commits into
umami-software:masterfrom
aladdyn-io:theopendraft

Conversation

@aladdyn-io

@aladdyn-io aladdyn-io commented Jun 9, 2026

Copy link
Copy Markdown

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

nishaaanth2 and others added 8 commits October 7, 2025 17:07
- Add new /api/websites/{websiteId}/all-stats endpoint
- Returns all website statistics in a single API call
- Reduces API calls from 10+ to 1 (60-75% performance improvement)
- Includes stats, session stats, timeseries, top metrics, and event data
- Fully backward compatible with existing endpoints
- Add complete documentation and usage examples
- Add TypeScript examples with React hooks
- Add testing guide and troubleshooting tips
- Add functionality to authenticate users with a special admin role using the SECRET_VALUE environment variable.
- Create an admin user object when the token matches SECRET_VALUE.
- Log authentication success in development mode for easier debugging.
@vercel

vercel Bot commented Jun 9, 2026

Copy link
Copy Markdown

@theopendraft is attempting to deploy a commit to the Umami Software Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a new /api/websites/{websiteId}/all-stats endpoint that aggregates all dashboard statistics into a single parallel query, along with CORS preflight handling for the send endpoint and several utility improvements. However, several of the accompanying changes to existing core files introduce serious defects.

  • src/lib/auth.ts adds a static env-var bearer token (SECRET_VALUE) that grants unconditional admin access, bypassing the entire JWT/session stack with no expiry or audit trail.
  • src/queries/prisma/website.ts silently replaces create with upsert in createWebsite, overwriting existing website data on ID collision instead of failing.
  • src/tracker/index.js removes keepalive: true from the tracker fetch, causing analytics events fired during page navigation/unload to be dropped by the browser.
  • src/app/api/websites/[websiteId]/all-stats/route.ts accesses result[0] without a null guard and passes an incomplete query object to getRequestDateRange, breaking the all-time date mode.

Confidence Score: 1/5

Not safe to merge — the auth module now contains a static master-credential bypass that grants permanent admin access to any caller who knows the env-var value.

The most consequential change is in src/lib/auth.ts, where a new code path short-circuits the entire JWT/session stack and hands out a synthetic admin identity to anyone presenting a matching static env-var token. Beyond that, createWebsite was silently converted to an upsert that can overwrite existing website data, the tracker lost keepalive: true causing analytics data loss on navigation, and the new all-stats endpoint has two runtime crash paths. Together these changes touch the authentication layer, the core data-write path, and the client-side tracking script in ways that would degrade or compromise a production deployment.

src/lib/auth.ts requires immediate attention for the auth bypass; src/queries/prisma/website.ts, src/tracker/index.js, and src/app/api/websites/[websiteId]/all-stats/route.ts each have defects that need to be resolved before the new endpoint is safe to ship.

Important Files Changed

Filename Overview
src/lib/auth.ts Introduces a static env-var-based admin bypass (SECRET_VALUE) that grants permanent, non-expiring admin access to any caller who presents the raw value as a Bearer token — a critical backdoor.
src/queries/prisma/website.ts Changes createWebsite from create to upsert, silently overwriting an existing website's name/domain on ID collision instead of failing — a data-integrity regression.
src/tracker/index.js Removes keepalive: true from the tracker's fetch call, causing analytics events fired during page unload/navigation to be silently dropped by the browser.
src/app/api/websites/[websiteId]/all-stats/route.ts New aggregated stats endpoint with two runtime bugs: unguarded [0] access on query results (crashes on empty data) and missing websiteId in the query object for all-time date range mode.
src/lib/request.ts Improves getJsonBody to handle empty bodies gracefully and rewrites getErrorMessages to recursively flatten nested Zod errors — both safe, quality improvements.
src/app/api/send/route.ts Adds an explicit OPTIONS handler for CORS preflight on the send endpoint; headers are appropriately scoped and the change is reasonable for Next.js 15 compatibility.
docker-compose.yml Silently changes the published host port from 3000 to 3010, which will break existing deployments and documentation.
next.config.mjs Only adds a // Forced reload comment at the end of the file; no functional change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GET /api/websites/:id/all-stats] --> B[parseRequest + Zod validation]
    B --> C{auth valid?}
    C -- No --> D[401 Unauthorized]
    C -- Yes --> E[canViewWebsite check]
    E -- No --> D
    E -- Yes --> F[getRequestDateRange]
    F --> G[getRequestFilters]
    G --> H[Promise.all - 13 parallel queries]
    H --> H1[getWebsiteStats current]
    H --> H2[getWebsiteStats prev period]
    H --> H3[getWebsiteSessionStats]
    H --> H4[getPageviewStats optional]
    H --> H5[getSessionStats optional]
    H --> H6[getPageviewMetrics url]
    H --> H7[getSessionMetrics x6]
    H --> H8[getEventDataStats]
    H1 & H2 & H3 & H4 & H5 & H6 & H7 & H8 --> I[Format stats + combine languages]
    I --> J[200 JSON response]

    subgraph Auth bypass added in this PR
    K[checkAuth] --> L{token == SECRET_VALUE?}
    L -- Yes --> M[Synthetic admin user - no DB lookup]
    L -- No --> N[Normal JWT validation]
    end
Loading

Comments Outside Diff (1)

  1. src/tracker/index.js, line 160-168 (link)

    P1 Removal of keepalive: true drops analytics on page navigation

    keepalive: true tells the browser to keep the request alive even after the current page unloads. Without it, any track() call that fires as part of a navigation — the most common analytics event — will be aborted by the browser before the server receives it. This causes silent data loss for every outbound link click, form submission, and SPA route change that the tracker observes. The option should be restored.

Reviews (1): Last reviewed commit: "deploy" | Re-trigger Greptile

Comment on lines 138 to 156
export async function createWebsite(
data: Prisma.WebsiteCreateInput | Prisma.WebsiteUncheckedCreateInput,
): Promise<Website> {
return prisma.client.website.create({
data,
// Ensure website_id is available in data
if (!data.id) {
throw new Error('id is required for upsert()');
}

return prisma.client.website.upsert({
where: { id: data.id },
update: {
// You can choose what to update on duplicate
name: data.name,
domain: (data as { domain?: string | null }).domain ?? null,
updatedAt: new Date(),
},
create: data,
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Silent website data overwrite on createWebsite

The original prisma.client.website.create() would throw a unique-constraint error on an ID collision — the correct fail-loud behaviour. The replacement upsert silently overwrites an existing website's name, domain, and updatedAt if the same id is submitted again, regardless of which user or team owns it. In a multi-tenant setup, a caller who knows or guesses an existing website UUID can rename or re-associate it without owning it. create is the correct primitive here; if idempotent creation is genuinely needed, it should be gated behind an explicit ownership check.

Comment on lines +124 to +138
const stats = Object.keys(websiteMetrics[0]).reduce((obj, key) => {
obj[key] = {
value: Number(websiteMetrics[0][key]) || 0,
prev: Number(prevWebsiteMetrics[0][key]) || 0,
};
return obj;
}, {} as Record<string, { value: number; prev: number }>);

// Format session stats
const sessionStatsData = Object.keys(sessionStats[0]).reduce((obj, key) => {
obj[key] = {
value: Number(sessionStats[0][key]) || 0,
};
return obj;
}, {} as Record<string, { value: number }>);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unguarded [0] access will throw on empty query results

websiteMetrics[0] and sessionStats[0] are accessed directly with no null/empty-array guard. If getWebsiteStats or getWebsiteSessionStats returns an empty array for a website with no data in the requested range, Object.keys(undefined) will throw a TypeError and crash the request with a 500. The response should check for an empty array before reducing, and fall back to a safe empty object.

return unauthorized();
}

const { startDate, endDate, unit } = await getRequestDateRange(query);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 websiteId missing from getRequestDateRange query object breaks all-time mode

getRequestDateRange reads query.websiteId to call getWebsiteDateRange when the all-time sentinel values (startAt=0, endAt=1) are used. In this route websiteId is a URL path parameter resolved via await params, so it is never present in the query object. Any call using the all-time mode will pass undefined to getWebsiteDateRange, which will likely throw a Prisma error. The resolved websiteId should be injected into the query object before passing it to getRequestDateRange.

Comment thread docker-compose.yml
image: ghcr.io/umami-software/umami:postgresql-latest
ports:
- '3000:3000'
- '3010:3000'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Port mapping changed from 3000 to 3010

The host port was silently changed from the documented default 3000:3000 to 3010:3000. This will break any existing deployments, scripts, or docs that reference port 3000. If intentional it should be called out explicitly in the PR description; otherwise it should be reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants